Skip to content

refactor: update document version handling and add SuperDoc ID [SD-428]#814

Merged
harbournick merged 15 commits intomainfrom
chore/new-superdoc-id
Oct 7, 2025
Merged

refactor: update document version handling and add SuperDoc ID [SD-428]#814
harbournick merged 15 commits intomainfrom
chore/new-superdoc-id

Conversation

@caio-pizzol
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol commented Aug 26, 2025

  • Renamed updateDocumentVersion to setDocumentVersion for clarity.
  • Introduced methods to get and set SuperDoc ID and version in SuperConverter.
  • Improved error handling and structure for custom properties in docx.
  • Added unit tests for new functionality in SuperConverter.

Notes:

  • MD5 hash (when GUID present)
  • Insert GUID on key stroke
  • Do not count on blank document

Copy link
Copy Markdown
Collaborator

@harbournick harbournick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and I like the ideas. Just a few comments for clarity

Comment thread packages/super-editor/src/core/Editor.js
Comment thread packages/super-editor/src/core/Editor.js Outdated
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js Outdated
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js Fixed

This comment was marked as outdated.

@caio-pizzol caio-pizzol requested a review from Copilot August 28, 2025 18:38

This comment was marked as outdated.

@caio-pizzol
Copy link
Copy Markdown
Contributor Author

@edoversb @harbournick I think we need a more deep analysis here before merging this

@caio-pizzol caio-pizzol requested a review from Copilot August 28, 2025 18:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors document identifier handling by introducing a new document GUID system alongside existing version management. The changes transition from a single document ID approach to separate handling of permanent GUIDs (for modified documents) and temporary hashes (for unmodified documents), improving telemetry tracking and document lifecycle management.

  • Renamed updateDocumentVersion to setDocumentVersion and improved error handling for custom properties
  • Added document GUID and identifier methods to SuperConverter with proper precedence (Microsoft GUID > custom GUID > hash)
  • Enhanced telemetry tracking with separate document GUID and identifier fields

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
shared/common/Telemetry.js Updated telemetry fields from single superdocId to separate documentGuid and documentIdentifier with hash generation removal
packages/super-editor/src/core/super-converter/exporter-docx-defs.js Removed unused SETTINGS_CUSTOM_XML export
packages/super-editor/src/core/super-converter/SuperConverter.test.js Added comprehensive test coverage for new GUID/identifier resolution and promotion functionality
packages/super-editor/src/core/super-converter/SuperConverter.js Major refactor adding document GUID management, identifier resolution, and generic custom property handling
packages/super-editor/src/core/Editor.js Added document modification tracking, GUID promotion, and telemetry data methods with backward compatibility
Comments suppressed due to low confidence (1)

packages/super-editor/src/core/super-converter/SuperConverter.js:1

  • The method signature has changed to add a new documentIdentifier parameter between existing parameters, which is a breaking change. Consider adding the new parameter at the end to maintain backward compatibility, or provide method overloading to handle both old and new signatures.
import xmljs from 'xml-js';

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js
Comment thread shared/common/Telemetry.js
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js
Comment thread packages/super-editor/src/core/Editor.js Outdated
Copy link
Copy Markdown
Collaborator

@harbournick harbournick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments. Overall it looks good to me. However main points:

  • Need to check if this works in collaboration
  • Need to test what happens if converter.docx = [] (ie: we are not in docx mode)
  • Please add unit tests to all new functions
  • We need to create a few test files to test the various cases (missing GUID etc). How can we add this to automated testing (either within this repo, or in the separate tests repo)

Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js Outdated
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js
Comment thread packages/super-editor/src/core/super-converter/SuperConverter.js Outdated
Comment thread packages/super-editor/src/core/Editor.js
@caio-pizzol caio-pizzol changed the base branch from develop to main September 1, 2025 14:17
@harbournick harbournick changed the title refactor: update document version handling and add SuperDoc ID refactor: update document version handling and add SuperDoc ID [SD-205] Oct 2, 2025
@linear
Copy link
Copy Markdown

linear bot commented Oct 2, 2025

@harbournick harbournick changed the title refactor: update document version handling and add SuperDoc ID [SD-205] refactor: update document version handling and add SuperDoc ID [SD-428] Oct 2, 2025
@linear
Copy link
Copy Markdown

linear bot commented Oct 2, 2025

…n methods

- Renamed `updateDocumentVersion` to `setDocumentVersion` for clarity.
- Introduced methods to get and set SuperDoc ID and version in `SuperConverter`.
- Improved error handling and structure for custom properties in docx.
- Added unit tests for new functionality in `SuperConverter`.
- Introduced a two-tier identification system for documents, utilizing a permanent GUID for modified documents and a temporary hash for unmodified ones.
- Updated methods to retrieve and promote document identifiers, ensuring backward compatibility with deprecated methods.
- Removed unused settings custom XML structure and streamlined document version handling.
- Enhanced telemetry to track document identifiers and updated related tests for comprehensive coverage.
- Eliminated the getDocumentId method to streamline the Editor class, as it is no longer needed following recent changes to document identifier handling.
- This refactor contributes to cleaner code and improved maintainability.
…Converter

- Updated the Editor class to simplify document modification tracking and promote to GUID when necessary.
- Refactored methods for retrieving document identifiers, ensuring clarity and consistency in handling GUIDs and hashes.
- Deprecated outdated methods for backward compatibility while enhancing telemetry data retrieval.
- Adjusted related tests to accommodate asynchronous changes in document identifier generation.
- Updated the SuperConverter class to safely access the 'Properties' element in custom XML, ensuring that the code handles cases where the element may not exist.
- This change improves the robustness of the document conversion process by preventing potential runtime errors.
- Added radix parameter to parseInt for clarity when converting PIDs.
- Replaced isFinite check with Number.isInteger to ensure PIDs are strictly integers, enhancing the robustness of the PID generation logic.
- Enhanced the getDocumentGuid method to parse XML instead of using regex for better accuracy and reliability.
- Implemented null checks to prevent errors when accessing XML elements, improving the robustness of the document conversion process.
- Included a comment in the SuperConverter class to clarify that the GUID is stored in custom properties during export to prevent unnecessary XML modifications if the document is not saved. This enhances code readability and understanding of the export process.
Copy link
Copy Markdown
Collaborator

@harbournick harbournick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@harbournick harbournick merged commit 25f91d2 into main Oct 7, 2025
7 checks passed
@harbournick harbournick deleted the chore/new-superdoc-id branch October 7, 2025 03:18
@superdocbot
Copy link
Copy Markdown
Contributor

superdocbot bot commented Oct 7, 2025

πŸŽ‰ This PR is included in version 0.23.0-next.15 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

@superdocbot
Copy link
Copy Markdown
Contributor

superdocbot bot commented Oct 10, 2025

πŸŽ‰ This PR is included in version 0.23.0 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants